Skip to content

ci: grant id-token: write for Slack notify workflow - #672

Merged
hln33 merged 1 commit into
mainfrom
fix/notify-id-token-permission
Aug 21, 2026
Merged

ci: grant id-token: write for Slack notify workflow#672
hln33 merged 1 commit into
mainfrom
fix/notify-id-token-permission

Conversation

@hln33

@hln33 hln33 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes aws/aws-durable-execution-ci#30.

The reusable notify.yml in aws-durable-execution-ci changed its
summarize job at ac55d9f to assume BEDROCK_ROLE_ARN via OIDC,
requiring id-token: write. Consumer notify.yml files still grant
only contents: read + models: read, and GitHub caps a reusable
workflow's token at the caller's grant — aborting the run at startup
and dropping all Slack notifications.

Changes (identical across all four consumer repos):

  • Add id-token: write permission
  • Explicitly forward BEDROCK_ROLE_ARN (enables AI-generated summaries
    when the org secret is configured) alongside the existing Slack webhooks

@hln33
hln33 force-pushed the fix/notify-id-token-permission branch from 452cb17 to f8fbbc1 Compare August 21, 2026 21:18
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 21, 2026 21:26 — with GitHub Actions Inactive
@hln33
hln33 temporarily deployed to ai-pr-review-runtime August 21, 2026 21:26 — with GitHub Actions Inactive
@hln33
hln33 merged commit 8a68c86 into main Aug 21, 2026
15 checks passed
@hln33
hln33 deleted the fix/notify-id-token-permission branch August 21, 2026 21:29
@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

No actionable findings. Residual risk is limited to the unexecuted integration with the pinned reusable workflow and repository secrets.

Reviewed commit f8fbbc1f7952adb14b1eb29af77d3eed63ce828d. Workflow run

@github-actions

Copy link
Copy Markdown
Contributor

Claude AI review

No blocking findings. This PR is a 3-line change to a single CI workflow (.github/workflows/notify.yml): it adds id-token: write and forwards BEDROCK_ROLE_ARN and SLACK_WEBHOOK_URL_DISCUSSION to the SHA-pinned reusable workflow. No SDK/runtime code is affected, so checkpoint/replay, serialization, and public-API concerns do not apply.

The id-token: write grant on a pull_request_target workflow is safe as written: pull_request_target always uses the base-branch definition of this caller (a fork cannot tamper with it), the reusable workflow is SHA-pinned (@ac55d9f), and the permission is scoped to the single notify job — the correct pattern for OIDC role assumption. BEDROCK_ROLE_ARN forwarding matches the stated rationale.

Residual risks worth confirming before merge (not confirmed defects, and not verifiable from this repo since the reusable workflow lives in aws/aws-durable-execution-ci):

  • Unused SLACK_WEBHOOK_URL_DISCUSSION / missing trigger (.github/workflows/notify.yml:24): the discussion webhook is now forwarded, but the on: block only declares pull_request_target, issues, and release — no discussion trigger. As-is, this workflow can never fire on a discussion event, so the forwarded secret is dead. If discussion notifications are intended for this repo, add a discussion trigger under on:; if this is only for cross-repo secret-signature parity, it is harmless but inert.
  • Secret declarations must exist at the pinned SHA: both newly forwarded secrets (BEDROCK_ROLE_ARN, SLACK_WEBHOOK_URL_DISCUSSION) must be declared under on.workflow_call.secrets in the reusable workflow at ac55d9f. If either is undeclared there, the run fails at startup with an input-is-not-defined error — the opposite of the PR intent. The PR description confirms BEDROCK_ROLE_ARN was added; please confirm SLACK_WEBHOOK_URL_DISCUSSION is likewise declared.

There are no automated tests for workflow YAML in this repo, so the only validation is the reusable-workflow contract above.

Reviewed commit f8fbbc1f7952adb14b1eb29af77d3eed63ce828d. Workflow run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Broken Slack notification workflow

2 participants